Show env normalization differences under two solvers #148939
Open
+112
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
adwinwhite
force-pushed
the
env_normalization_fcw
branch
from
5cd4303 to
4408f14
Compare
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
lcnr
reviewed
Nov 14, 2025
Comment on lines
220
to
221
| // FIXME: `generic_const_exprs` causes query cycle problem. Same as the old solver. | ||
| if !ct.has_aliases() || infcx.tcx.features().generic_const_exprs() { |
Contributor
There was a problem hiding this comment.
hmm, disable the lint if generic_const_exprs is enabled instead
Contributor
Author
There was a problem hiding this comment.
Ah! That's better. generic_const_exprs is not compatible with next solver anyway.
lcnr
reviewed
Nov 16, 2025
| orig_pred, | ||
| ) { | ||
| Ok(pred_by_next) => { | ||
| if pred_by_next == *pred_by_old { |
Contributor
There was a problem hiding this comment.
need to fully resolve the predicates before comparing them. This is causing the error in std. Also just generally move this further to the end of this function?
I think doing it here isn't ideally, but should be good enough for a crater run
Contributor
Author
There was a problem hiding this comment.
std compiles fine after resolving predicates normalized by the old solver.
It turns out that stderr changes of some tests are noise. They're restored to the original state.
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Contributor
Author
|
@rustbot author |
rustbot
added
S-waiting-on-author
Collaborator
|
Reminder, once the PR becomes ready for a review, use |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First step for param_env normalization future compat warning. zulip thread
I didn't put the check directly in
param_envquery becausenormalize_param_env_or_errorhas some adjustments on the predicates: elaboration and outlive predicates manipulation. I'd have to replicate these inparam_envto compare normalized predicates properly.The downside of putting the check inside
normalize_param_env_or_erroris that it's used in more thanparam_envquery. It's also used incompare_impl_itemand several other places. I'm not sure if this is desired.I didn't bless tests since the hard error will be changed to lint later.Blessed tests to demonstrate the changes.
And there's one test about const generics I don't know how to fix.
Canonicalizer for the next solver will replace
ParamConstwithPlaceholderConst, but it still uses the sametry_evaluate_constinternally andprocess_obligationdoesn't likePlaceholderConst.r? @lcnr